env var to disable all middleware spans by bengl · Pull Request #5044 · DataDog/dd-trace-js

bengl · 2024-12-19T13:47:28Z

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

github-actions · 2024-12-19T13:48:25Z

Overall package size

Self size: 8.55 MB
Deduped: 94.94 MB
No deduping: 95.45 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.4.0 | 29.44 MB | 29.44 MB | | @datadog/native-appsec | 8.4.0 | 19.25 MB | 19.26 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.5.1 | 9.79 MB | 10.17 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.6.1 | 2.59 MB | 2.73 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.1.0 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

pr-commenter · 2025-01-08T02:45:38Z

Benchmarks

Benchmark execution time: 2025-01-30 19:09:30

Comparing candidate commit 57d7c10 in PR branch bengl/disable-all-middleware-option with baseline commit bfdd3ad in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 911 metrics, 21 unstable metrics.

scenario:plugin-graphql-with-depth-and-collapse-on-18

🟩 max_rss_usage [-69.458MB; -56.506MB] or [-7.119%; -5.791%]

codecov · 2025-01-15T16:27:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.07%. Comparing base (bfdd3ad) to head (57d7c10).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5044   +/-   ##
=======================================
  Coverage   81.07%   81.07%           
=======================================
  Files         479      479           
  Lines       21337    21342    +5     
=======================================
+ Hits        17298    17303    +5     
  Misses       4039     4039

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rochdev · 2025-01-30T09:21:37Z

Altering environment variables should be done in beforeEach and afterEach to make sure that the previous value is restored.

rochdev · 2025-01-30T09:22:52Z

The environment variable doesn't match the name of the programmatic option.

It also doesn't have a corresponding test.

The environment variable doesn't match the name of the programmatic option.

Please provide suggested values for both. @tlhunter @rochdev please coordinate on this because right now you folks have both made naming suggestions.

It also doesn't have a corresponding test.

I don't understand this comment. In this same review, you commented on the corresponding test. What additional testing are you looking for?

Please provide suggested values for both.

I don't mind the name, but it should map between programmatic and env var.

I don't understand this comment. In this same review, you commented on the corresponding test.

The plugin test yes, but there isn't a test for the option itself (config.spec.js)

I don't mind the name, but it should map between programmatic and env var.

So renaming the env var to DD_TRACE_MIDDLEWARE_TRACING_ENABLED would be sufficient? That's a lot of TRACE in the name, but I guess that's fine.

That's a lot of TRACE in the name, but I guess that's fine.

We did name the library dd-trace after all 😅

rochdev · 2025-01-30T09:24:15Z

This should be done in the plugin manager, consistent with other similar options. While it could be argued that this is not ideal, changing how this works is out of scope of this PR.

datadog-datadog-prod-us1 · 2025-01-30T17:18:41Z

Datadog Report

Branch report: bengl/disable-all-middleware-option
Commit report: 64eff95
Test service: dd-trace-js-integration-tests

✅ 0 Failed, 614 Passed, 0 Skipped, 13m 59.93s Total Time

…pec.js Co-authored-by: Bryan English <bryan.english@datadoghq.com>

* env var to disable all middleware spans --------- Co-authored-by: Ida.Liu <ida.liu@datadoghq.com> Co-authored-by: Ida Liu <119438987+ida613@users.noreply.github.com>

ida613 added the semver-minor label Jan 8, 2025

bengl force-pushed the bengl/disable-all-middleware-option branch from afddc71 to 48d8784 Compare January 15, 2025 15:52

bengl commented Jan 23, 2025

View reviewed changes

Comment thread packages/datadog-plugin-express/test/integration-test/client.spec.js Outdated

tlhunter reviewed Jan 23, 2025

View reviewed changes

Comment thread packages/dd-trace/src/config.js Outdated

tlhunter previously approved these changes Jan 28, 2025

View reviewed changes

bengl dismissed tlhunter’s stale review via eefbd9f January 29, 2025 16:23

tlhunter previously approved these changes Jan 29, 2025

View reviewed changes

bengl force-pushed the bengl/disable-all-middleware-option branch from eefbd9f to 830d10a Compare January 29, 2025 20:44

bengl marked this pull request as ready for review January 29, 2025 20:45

bengl requested review from a team as code owners January 29, 2025 20:45

bengl enabled auto-merge (squash) January 29, 2025 20:45

bengl dismissed tlhunter’s stale review via dc9f5d4 January 29, 2025 21:14

tlhunter previously approved these changes Jan 29, 2025

View reviewed changes

bengl dismissed tlhunter’s stale review via 794fe56 January 29, 2025 21:37

tlhunter previously approved these changes Jan 29, 2025

View reviewed changes

rochdev requested changes Jan 30, 2025

View reviewed changes

bengl dismissed tlhunter’s stale review via 07b46a2 January 30, 2025 17:13

rochdev approved these changes Jan 30, 2025

View reviewed changes

bengl and others added 7 commits January 30, 2025 13:59

env var to disable all middleware spans

9e22919

fix linter errors

196d940

adding tracer config option for middleware

0cb6f4a

separate middleware tests into different files

6d2d58a

fix linter errors

3534548

convert unit test to integration test

d964761

fix integration test

2c4e1fd

ida613 and others added 8 commits January 30, 2025 13:59

fix integration test

c3269b3

Update packages/datadog-plugin-express/test/integration-test/client.s…

f33f38d

…pec.js Co-authored-by: Bryan English <bryan.english@datadoghq.com>

change middleware config value name

3797523

enableMiddlewareTracing -> middlewareTracingEnabled

45a517f

properly change config option name

d9fac79

lint

90cc1f3

try again to make the tests pass

79b0699

PR review fixes

57d7c10

bengl force-pushed the bengl/disable-all-middleware-option branch from 07b46a2 to 57d7c10 Compare January 30, 2025 18:59

bengl merged commit e8e96bf into master Jan 30, 2025

bengl deleted the bengl/disable-all-middleware-option branch January 30, 2025 19:12

watson pushed a commit that referenced this pull request Jan 31, 2025

env var to disable all middleware spans (#5044)

cfa59ca

* env var to disable all middleware spans --------- Co-authored-by: Ida.Liu <ida.liu@datadoghq.com> Co-authored-by: Ida Liu <119438987+ida613@users.noreply.github.com>

watson mentioned this pull request Jan 31, 2025

v5.34.0 proposal #5184

Merged

watson pushed a commit that referenced this pull request Jan 31, 2025

env var to disable all middleware spans (#5044)

fafa12c

* env var to disable all middleware spans --------- Co-authored-by: Ida.Liu <ida.liu@datadoghq.com> Co-authored-by: Ida Liu <119438987+ida613@users.noreply.github.com>

Conversation

bengl commented Dec 19, 2024

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

Uh oh!

github-actions Bot commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Uh oh!

pr-commenter Bot commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

scenario:plugin-graphql-with-depth-and-collapse-on-18

Uh oh!

codecov Bot commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datadog-datadog-prod-us1 Bot commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Dec 19, 2024 •

edited

Loading

pr-commenter Bot commented Jan 8, 2025 •

edited

Loading

codecov Bot commented Jan 15, 2025 •

edited

Loading

datadog-datadog-prod-us1 Bot commented Jan 30, 2025 •

edited

Loading